Add project-scoped OAuth authorization - #138
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
47f1631 to
1d265ac
Compare
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1d265ac. Configure here.
There was a problem hiding this comment.
reviewed the latest version — the atomic token persistence fix looks good. requesting changes for the first two OAuth correctness issues; the rest could be follow-ups.
bugs
src/app/token/route.ts:236-266,291-299— after verifying a legacy refresh context, persist an enriched context containingpayload.sub. currently the rotated token retains noclerk_user_id, so active legacy clients stay on the post-exchange membership path indefinitely and a transient failure can still burn their refresh chain.src/app/authorize/route.ts:217-224— non-PKCE organization grants store one context per publicclient_id, whilesrc/lib/org-utils.ts:99-103retrieves that last-write-wins value. concurrent users or flows can overwrite each other. can we restrict this compatibility path to an explicit legacy allowlist and require S256 PKCE for all other clients?
follow-ups
src/app/select-org/page.tsx:360-393,519-527— the new scope picker hard-codes light surfaces while using dark-mode-aware text tokens, producing low-contrast text under system dark mode. use semantic surface and border tokens or explicitly keep this page light.src/lib/oauth-projects.ts:60-63,106-109andsrc/app/token/route.ts:188-209— thread request cancellation and an explicit timeout through the new Kernel and Clerk calls so abandoned or stalled OAuth requests do not continue consuming server work.src/lib/oauth-projects.ts:73,121— normalize successful-response parse/schema failures and fetch exceptions toOAuthProjectsError(502); otherwise/authorizereports upstream failures as “project not found or inactive.”
|
addressed both blocking OAuth correctness issues in 5970522:
added regression coverage for non-allowlisted clients, allowlisted organization grants, rejected project contexts, and legacy-context enrichment. |
hiroTamada
left a comment
hiroTamada
left a comment
There was a problem hiding this comment.
latest fixes resolve the two OAuth correctness blockers. the remaining notes are non-blocking follow-ups. 173 tests and TypeScript pass locally.
|
addressed the remaining review follow-ups that apply to this rollout:
request cancellation and timeout plumbing remains deferred as a separate reliability follow-up. |
2 participants
summary
tests
bun test(175 passing)bun run buildwith test build-time configurationdependency
Requires kernel/kernel#3165 to be deployed before this change writes structured OAuth contexts.
Note
High Risk
Changes core OAuth authorize/token flows, Redis persistence shape, and scope enforcement; depends on kernel#3165 for structured contexts and misconfiguration could break login or widen/narrow access incorrectly.
Overview
Adds organization-wide vs single-project OAuth authorization, with project choice in
/select-org(search/paginated via newGET /oauth/projects) and scope carried through authorize → token responses.Authorization context replaces org-only Redis mappings: structured JSON (org + optional
project_id) is stored on PKCE-bound requests, JWT/refresh tokens, and consumed atomically on code exchange. S256 PKCE is required for new clients and mandatory for project scope;OAUTH_LEGACY_NON_PKCE_CLIENT_IDSkeeps org-only flows for allowlisted clients.Token route resolves scope server-side (ignores client-supplied
org_id/access_scope/project_id), checks Clerk org membership before refresh rotation, validates subject on auth-code grants, and returnsorg_id,access_scope, andproject_idon issued tokens.Docs/env note the legacy allowlist; layout forces light theme. Broad route and lib test coverage added.
Reviewed by Cursor Bugbot for commit 010e8a0. Bugbot is set up for automated code reviews on this repo. Configure here.